pwm3360 driver cleanup and diff reduction to adns9800 #15559
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
drashna
requested changes
Dec 23, 2021
drivers/sensors/pmw3360.c
Outdated
| @@ -86,23 +89,18 @@ void print_byte(uint8_t byte) { dprintf("%c%c%c%c%c%c%c%c|", (byte & 0x80 ? '1' | |||
| #endif | |||
| #define constrain(amt, low, high) ((amt) < (low) ? (low) : ((amt) > (high) ? (high) : (amt))) | |||
|
|
|||
| bool spi_start_adv(void) { | |||
| bool status = spi_start(PMW3360_CS_PIN, PMW3360_SPI_LSBFIRST, PMW3360_SPI_MODE, PMW3360_SPI_DIVISOR); | |||
| void pmw3360_spi_start(void) { | |||
There was a problem hiding this comment.
Please revert this to a bool, and keep the return status. There are some improvements/better handling I have planned that does depend on this.
There was a problem hiding this comment.
Could you outline roughly what you have planned? Should we change all pointing_device_driver_t to return a bool?
So I'm trying to add multi-device support, and was thinking of having the user set:
#define PMW3360_CS_PINS { B5, B6 }
and then just looping over them for init and firmware upload, and merging their reports either in quantum/pointing_device_drivers.c or in the driver itself. Does that sound like a useful approach?
There was a problem hiding this comment.
I think discussion for adding multiple sensors should be added later.
But the plans I have are to basically block queries if there are errors. That way, it isn't constantly calling the sensor code if it isn't connected or is disconnected.
but pin vs pins is probably the best way to go about it. But it will require a significant rewrite to the code, I think.
|
Also, I'm confused, should this go into master or develop? |
This should go into develop. Any sort of core changes should. Unless it's a bug fix (which this isn't) |
They are very similar devices. This (somewhat) unreadable diff is essentially a no-op, but it makes a `vimdiff` between the 2 drivers much more readable.
Remove redundant calls to spi_start() and spi_stop(), as pmw3360_write() will already call these.
|
Rebased onto develop now, this should be good to go. |
drashna
approved these changes
Dec 23, 2021
tzarc
approved these changes
Dec 27, 2021
roccojiang
added a commit
to roccojiang/qmk_firmware
that referenced
this pull request
Feb 26, 2022
* Start `develop` for 2022q1. * Added cancel_key_lock function (qmk#15321) * [Core] Remove matrix_is_modified() and debounce_is_active() (qmk#15349) * [Keyboard] Added Wakizashi 40 (qmk#15336) Co-authored-by: Ryan <[email protected]> * Change default USB Polling rate to 1kHz (qmk#15352) * [Keyboard] Convert ergoinu to SPLIT_KEYBOARD (qmk#15305) * Implement MAGIC_TOGGLE_CONTROL_CAPSLOCK (qmk#15368) * Convert not_so_minidox to SPLIT_KEYBOARD (qmk#15306) * Convert ai03/orbit to SPLIT_KEYBOARD (qmk#15340) * Tidy up existing i2c_master implementations (qmk#15376) * Move chibios defines out of header * Make some avr defines internal * Generalize Unicode defines (qmk#15409) * Add missing define for unicode common (qmk#15416) * Remove Deprecated USB Polling comment from vusb.c (qmk#15420) * Expand rotational range for PMW3360 Optical Sensor (qmk#15431) * Add support for 21.11.x, remove 21.6.x as ChibiOS "canceled" it. (qmk#15435) * added missing audio_off_user() callback (qmk#15457) Co-authored-by: Raoul Rubien <[email protected]> * [Core] Don't send keyboard reports that propagate no changes to the host (qmk#14065) * Migrate serial_uart usages to UART driver (qmk#15479) * Migrate Thermal Printer feature to UART driver * Migrate 40percentclub UT47 to UART driver * Migrate Centromere to UART driver * Migrate Chimera Ergo to UART driver * Migrate Chimera Let's Split to UART driver * Migrate Chimera Ortho to UART driver * Migrate Chimera Ortho Plus to UART driver * Migrate Comet46 to UART driver * Migrate Palm USB converter to UART driver * Migrate Sun USB converter to UART driver * Migrate Dichotomy to UART driver * Migrate Honeycomb to UART driver * Migrate Mitosis to UART driver * Migrate Redox W to UART driver * Migrate Uni660 to UART driver * Migrate Telophase to UART driver * Fix build failure for UT47 (qmk#15483) * Use the PR title rather than parsing the commit message. (qmk#15537) * Migrate RN42 to UART driver and refactor (qmk#15492) * [CI] Format code according to conventions (qmk#15541) * Documentation Typo fix (qmk#15538) * Fix some typos, especially the sensor name. (qmk#15557) * Add open-drain GPIO support. (qmk#15282) * Add open-drain GPIO support. * `qmk format-c` * Wording. * Remove port GPIO implementations as the only board that uses it has its own internal defs anyway. Will wait for first-class handling of ports in core before reimplementing. * Fixes potential wpm sampling overflow, along with code comment fixes (qmk#15277) Co-authored-by: Trevor Powell <[email protected]> * Add a clarification to an error message (qmk#15207) Makes this a bit more foolproof. See qmk#15202 * [Core] Split support for pointing devices. (qmk#15304) * Draft implementation * formatting * fix combined buttons * remove pimoroni throttle * sync pointing on a throttle loop with checksum * no longer used * doh Co-authored-by: Drashna Jaelre <[email protected]> * switch pimoroni to a cpi equivalent * add cpi support * allow user modification of seperate mouse reports * a little tidy up * add *_RIGHT defines. * docs * doxygen comments * basic changelog * clean up pimoroni * small doc fixes * Update docs/feature_pointing_device.md Co-authored-by: Drashna Jaelre <[email protected]> * performance tweak if side has usb * Don't run init funtions on wrong side * renamed some variables for consistency * fix pimoroni typos * Clamp instead of OR * Promote combined values to uint16_t * Update pointing_device.c Co-authored-by: Drashna Jaelre <[email protected]> Co-authored-by: Nick Brassel <[email protected]> * Format code according to conventions (qmk#15588) * Make (un)register code functions weak (qmk#15285) * Defer pin operations to gpio.h (qmk#15589) * Add sym_defer_pr debouncer type (qmk#14948) * Durgod: Increase scan rate by using wait_us GPT timer (qmk#14091) Lower the tick rate from 10kHz to 1kHz (otherwise all the extra interrupts reduce the achievable scan rate). Enable the WAIT_US_TIMER using GPT TIM3. Observed scan rate on the K320 is increased from 625Hz to 2090-2120Hz. * Format code according to conventions (qmk#15590) * Fixup line endings * More GPIO compilation fixes. (qmk#15592) * Custom matrix lite support for split keyboards (qmk#14674) * Custom matrix lite support for split keyboards * WIP: matrix -> matrix_common refactor * Move matrix_post_scan() to matrix_common.c * Refactor `bootloader_jump()` implementations (qmk#15450) * Refactor `bootloader_jump()` implementations * Fix tests? * Rename `atmel-samba` to `md-boot` * [Keymap] Add vitoni keymap for GMMK Pro (ISO) (qmk#15006) * [Keymap] Add vitoni layout for GMMK Pro (ISO) Keymap has layered cursor keys similar to laptop keyboards. * Configure RGB defaults for startup * Configure encoder to change value/brightness on FN layer * Remove FN layer and add dedicated RGB layer * Make RGB layer sticky (using TG) to avoid holding FN while configuring RGB * Add RGB indicators for active layers * Add RGB indicator for active RESET mode Signed-off-by: Victor Toni <[email protected]> * Configure idle / USB suspend settings * Add RGB fade in when resuming after suspend * Add RGB fade out before suspend * Add fade out before idle * Add breathe effect when idle * Convert some more boards to Matrix Lite (qmk#15489) * Fix for SPI write timing in PMW3360 driver (qmk#15519) Timing does not match Pixart documentation for this sensor (may have been carried forward from adns9800). Not aware of any issues coming from this currently. It should only cause issues when writing to multiple registers in succession which currently only happens during initialization for the PMW3360. This should prevent future issues with write operations if other features of the sensor are added. * Format code according to conventions (qmk#15593) * pwm3360 driver cleanup and diff reduction to adns9800 (qmk#15559) * Diff reduction between ADNS9800 and PMW3360 drivers. They are very similar devices. This (somewhat) unreadable diff is essentially a no-op, but it makes a `vimdiff` between the 2 drivers much more readable. * Cleanup pwm3360 driver some more. Remove redundant calls to spi_start() and spi_stop(), as pmw3360_write() will already call these. * [Core] Fix bug and code regression for Split Common (qmk#15603) * Format code according to conventions (qmk#15604) * [Bug] Include missing string.h include (qmk#15606) * move bm65hsrgb_iso and bm68hsrgb to rev1/ (qmk#15132) * Move to organization folder (qmk#15481) * move directory * fix * move wings42 to dailycraft * Adjust wings42 to work with organization folder. * [Keyboard] Update Tractyl Manuform to use Split Pointing Device Sync * [Keyboard] Update grs_70ec to use newer custom matrix (qmk#15609) * [Keyboard] Fix compiler issue with tractyl manuform 4x6 (qmk#15646) * Fix split pointing for analog joystick (qmk#15691) Co-authored-by: Nick Brassel <[email protected]> * Format code according to conventions (qmk#15693) * Update pmw3360 comments to match the datasheet better, fix delays. (qmk#15682) * move @yangdigi 's keyboards to a YDKB folder (qmk#15681) * Format code according to conventions (qmk#15705) Co-authored-by: Nick Brassel <[email protected]> Co-authored-by: QMK Bot <[email protected]> Co-authored-by: wheredoesyourmindgo <[email protected]> Co-authored-by: Stefan Kerkmann <[email protected]> Co-authored-by: xiao <[email protected]> Co-authored-by: Ryan <[email protected]> Co-authored-by: Drashna Jaelre <[email protected]> Co-authored-by: Joel Challis <[email protected]> Co-authored-by: Dipl.-Ing. Raoul Rubien, BSc <[email protected]> Co-authored-by: Raoul Rubien <[email protected]> Co-authored-by: SmollChungus <[email protected]> Co-authored-by: uqs <[email protected]> Co-authored-by: vectorstorm <[email protected]> Co-authored-by: Trevor Powell <[email protected]> Co-authored-by: Hugo Osvaldo Barrera <[email protected]> Co-authored-by: Dasky <[email protected]> Co-authored-by: Chad Austin <[email protected]> Co-authored-by: Simon Arlott <[email protected]> Co-authored-by: Jay Greco <[email protected]> Co-authored-by: Victor Toni <[email protected]> Co-authored-by: Alabastard-64 <[email protected]> Co-authored-by: peepeetee <[email protected]> Co-authored-by: yfuku <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
While working on the driver to add multi-device support, I noticed that it's very similar to adns9800 and to be able to better read them side-by-side I've renamed and shuffled things around a bit.
This removes unused defines, unused includes, unused return codes and renames a lot of things (sorry!).
It also removes some redundant calls to start/stop that are entirely duplicated.
Types of Changes
Issues Fixed or Closed by This PR
Checklist